feat(flex-linux-setup): Admin UI and Cedarling integration#2407
feat(flex-linux-setup): Admin UI and Cedarling integration#2407
Conversation
Signed-off-by: Mustafa Baser <mbaser@mail.com>
devrimyatar
added
comp-flex-linux-setup
📝 WalkthroughWalkthroughAdds Cedarling policy-store support to Admin UI setup: new policy-store template, admin UI config keys, LDIF resource-scope mappings, and install steps to render the policy store, set ownership, and import mappings during Admin UI installation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Installer as flex_setup.install_gluu_admin_ui
participant Templates as templates/
participant FS as AdminUI config dir
participant DB as dbUtils (LDAP/DB)
rect rgb(222,235,247)
Installer->>Templates: read policy-store.json template
Installer->>FS: render policy-store.json -> ./custom/config/adminUI/policy-store.json
Installer->>FS: chown ./custom/config/adminUI -> jetty:jetty
Installer->>DB: import adminUIResourceScopesMapping.ldif
DB-->>Installer: import result (success/failure)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
flex-linux-setup/flex_linux_setup/flex_setup.py(2 hunks)flex-linux-setup/flex_linux_setup/templates/adminUIResourceScopesMapping.ldif(1 hunks)flex-linux-setup/flex_linux_setup/templates/auiConfiguration.json(1 hunks)flex-linux-setup/flex_linux_setup/templates/policy-store.json(1 hunks)
🔇 Additional comments (6)
flex-linux-setup/flex_linux_setup/flex_setup.py (3)
327-327: LGTM!The policy store path is correctly constructed relative to the templates directory.
572-574: Verify ifrenderTemplateInOutcreates directories before rendering.The implementation of
renderTemplateInOutis in an external dependency (setup_app.installers.config_api), which is not available in this repository for inspection. However, the codebase shows an inconsistent pattern: at lines 625–627, explicit directory creation is performed before achownoperation, while lines 572–574 callrenderTemplateInOutwithout explicit directory creation.Since the external module implementation cannot be verified here, manually confirm whether
renderTemplateInOuthandles directory creation internally, or add an explicit check (matching the pattern at lines 625–627) to ensure robustness.
575-577: Let me verify the import sequence and whether the parent OU is created elsewhere:Based on my comprehensive verification, the review comment raises valid and critical concerns that are confirmed by the codebase examination:Findings:
- The
adminUIResourceScopesMapping.ldiffile contains 40+ entries referencingou=adminUIResourceScopesMapping,ou=admin-ui,o=jans- The parent organizational unit
ou=adminUIResourceScopesMappingis never explicitly created in any LDIF file in the codebase- The
aui_webhook.ldifcreatesou=admin-ui,o=jansand child OUs, but notou=adminUIResourceScopesMapping- The objectClass schema definition for
adminUIResourceScopesMappingis not found in the codebase (likely expected to be pre-defined in the LDAP server)- LDAP import will fail with "parent does not exist" error if the parent OU is not created before importing
The review comment is accurate and the concern is substantive.
Verify LDAP schema supports adminUIResourceScopesMapping objectClass.
The LDIF import at line 577 requires the parent DN
ou=adminUIResourceScopesMapping,ou=admin-ui,o=jansto exist before importing. The parent OU is not created inadminUIResourceScopesMapping.ldifor any other LDIF file in the codebase. Additionally, the objectClass schema definition is not present. Ensure the parent organizational unit is created in the proper sequence (e.g., add it toaui_webhook.ldifor appropriate setup initialization) and that the LDAP server has theadminUIResourceScopesMappingschema loaded.flex-linux-setup/flex_linux_setup/templates/policy-store.json (2)
39-88: LGTM!The trusted issuer configuration with token metadata for access_token, id_token, and userinfo_token is well-structured. The
%(hostname)splaceholder will be correctly rendered byrenderTemplateInOutinflex_setup.pyline 573.
7-37: LGTM!The policy definitions for admin and auditor roles are well-structured. The base64-encoded policy content follows Cedar policy language format, and the descriptions clearly indicate the authorization rules (e.g., admin can manage auth server configuration, auditor can manage system monitoring and clients).
flex-linux-setup/flex_linux_setup/templates/adminUIResourceScopesMapping.ldif (1)
1-595: No issues found with the LDIF file structure.Verification confirmed that all 60 resource-scope mapping entries have unique inums with zero duplicates. The file is structurally sound and ready for LDAP import.
| { | ||
| "cedar_version": "4.4.0", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Cedar version compatibility with Cedarling.
The policy store specifies cedar_version: "4.4.0". Ensure this version is compatible with the Cedarling integration in the Admin UI to avoid runtime policy evaluation errors.
🌐 Web query:
What version of Cedar policy language does Cedarling support?
💡 Result:
Cedarling supports Cedar policy language version v4.0.0. [1][2]
Sources:
[1] Janssen docs — Cedarling Policy Store (stable).
[2] Janssen docs — Cedarling Policy Store (v1.8.0 example).
Update cedar_version to match Cedarling support.
Cedarling supports Cedar policy language version v4.0.0, but the policy store specifies cedar_version: "4.4.0". This mismatch will cause runtime policy evaluation errors. Change line 2 to "cedar_version": "4.0.0".
🤖 Prompt for AI Agents
In flex-linux-setup/flex_linux_setup/templates/policy-store.json around lines 1
to 2, the cedar_version is set to "4.4.0" but Cedarling supports v4.0.0; update
the value on line 2 to "4.0.0" so the policy store matches Cedarling's supported
Cedar policy language version.
Signed-off-by: Mustafa Baser <mbaser@mail.com>
Signed-off-by: Mustafa Baser <mbaser@mail.com>
Signed-off-by: Mustafa Baser <mbaser@mail.com>
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
flex-linux-setup/flex_linux_setup/templates/policy-store.json (1)
1-2: Cedar version compatibility issue persists.The past review correctly identified that Cedarling supports Cedar policy language version v4.0.0, but Line 2 still specifies
"cedar_version": "4.4.0". This version mismatch will cause runtime policy evaluation errors.Apply this diff to fix:
- "cedar_version": "4.4.0", + "cedar_version": "4.0.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
flex-linux-setup/flex_linux_setup/flex_setup.py(2 hunks)flex-linux-setup/flex_linux_setup/templates/auiConfiguration.json(1 hunks)flex-linux-setup/flex_linux_setup/templates/policy-store.json(1 hunks)
🔇 Additional comments (3)
flex-linux-setup/flex_linux_setup/templates/auiConfiguration.json (1)
38-41: LGTM! Policy store configuration correctly integrated.The new policy store configuration fields are properly structured:
auiDefaultPolicyStorePathcorrectly references./custom/config/adminUI/policy-store.json, which matches the filename rendered by the installer inflex_setup.py(line 327 definespolicy-store.jsonas the template name).- The remote vs. local policy store toggle is appropriately defaulted to local (
useRemotePolicyStore: false).This resolves the filename mismatch issue flagged in the previous review.
flex-linux-setup/flex_linux_setup/flex_setup.py (2)
327-327: LGTM! Policy store path correctly defined.The
policy_store_pathattribute correctly points topolicy-store.json, which matches the references inauiConfiguration.json(line 40) and ensures consistent naming throughout the cedarling integration.
571-574: LGTM! Policy store rendering and permissions properly handled.The cedarling integration correctly:
- Renders the policy store template into the Admin UI config directory
- Sets appropriate ownership for the jetty user to access the policy store
| resource_scopes_mapping_lidf_fn = os.path.join(self.templates_dir, 'adminUIResourceScopesMapping.ldif') | ||
|
|
||
| self.dbUtils.import_ldif([resource_scopes_mapping_lidf_fn]) |
There was a problem hiding this comment.
Fix variable naming and add file existence check.
Two issues:
- Line 575: Variable name
resource_scopes_mapping_lidf_fncontains a typo—should beldif_fn(LDAP Data Interchange Format, not "lidf"). - Line 577: No check if the LDIF file exists before importing, which could cause a runtime error if the file is missing.
Apply this diff:
- resource_scopes_mapping_lidf_fn = os.path.join(self.templates_dir, 'adminUIResourceScopesMapping.ldif')
+ resource_scopes_mapping_ldif_fn = os.path.join(self.templates_dir, 'adminUIResourceScopesMapping.ldif')
- self.dbUtils.import_ldif([resource_scopes_mapping_lidf_fn])
+ if os.path.exists(resource_scopes_mapping_ldif_fn):
+ self.dbUtils.import_ldif([resource_scopes_mapping_ldif_fn])
+ else:
+ print(f"Warning: LDIF file not found: {resource_scopes_mapping_ldif_fn}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resource_scopes_mapping_lidf_fn = os.path.join(self.templates_dir, 'adminUIResourceScopesMapping.ldif') | |
| self.dbUtils.import_ldif([resource_scopes_mapping_lidf_fn]) | |
| resource_scopes_mapping_ldif_fn = os.path.join(self.templates_dir, 'adminUIResourceScopesMapping.ldif') | |
| if os.path.exists(resource_scopes_mapping_ldif_fn): | |
| self.dbUtils.import_ldif([resource_scopes_mapping_ldif_fn]) | |
| else: | |
| print(f"Warning: LDIF file not found: {resource_scopes_mapping_ldif_fn}") |
🤖 Prompt for AI Agents
In flex-linux-setup/flex_linux_setup/flex_setup.py around lines 575 to 577, the
variable name resource_scopes_mapping_lidf_fn has a typo (should be ldif_fn) and
there is no check that the LDIF file exists before importing; rename the
variable to resource_scopes_mapping_ldif_fn and before calling
self.dbUtils.import_ldif(...) verify the file exists with
os.path.exists(resource_scopes_mapping_ldif_fn) (or os.path.isfile) and handle
the missing file by logging an error and exiting or raising an exception so
import_ldif is only called with a valid path.
2 participants
Closes #2399
Summary by CodeRabbit